-
Notifications
You must be signed in to change notification settings - Fork 637
Change ECS Agent log rollover defaults to size-based rotation #4776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1bfef12
to
050e301
Compare
050e301
to
395818c
Compare
mye956
reviewed
Sep 23, 2025
README.md
Outdated
| `ECS_LOG_ROLLOVER_TYPE` | `size` | `hourly` | Determines whether the container agent logfile will be rotated based on size or hourly. By default, the agent logfile is rotated based on size. | `size` | `size` | | ||
| `ECS_LOG_OUTPUT_FORMAT` | `logfmt` | `json` | Determines the log output format. When the json format is used, each line in the log would be a structured JSON map. | `logfmt` | `logfmt` | | ||
| `ECS_LOG_MAX_FILE_SIZE_MB` | `10` | When the ECS_LOG_ROLLOVER_TYPE variable is set to size, this variable determines the maximum size (in MB) the log file before it is rotated. If the rollover type is set to hourly then this variable is ignored. | `10` | `10` | | ||
| `ECS_LOG_MAX_FILE_SIZE_MB` | `1` | When the ECS_LOG_ROLLOVER_TYPE variable is set to size, this variable determines the maximum size (in MB) the log file before it is rotated. If the rollover type is set to hourly then this variable is ignored. | `1` | `1` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we say the default value is 5mb
here?
harishxr
reviewed
Sep 23, 2025
README.md
Outdated
| `ECS_LOG_ROLLOVER_TYPE` | `size` | `hourly` | Determines whether the container agent logfile will be rotated based on size or hourly. By default, the agent logfile is rotated based on size. | `size` | `size` | | ||
| `ECS_LOG_OUTPUT_FORMAT` | `logfmt` | `json` | Determines the log output format. When the json format is used, each line in the log would be a structured JSON map. | `logfmt` | `logfmt` | | ||
| `ECS_LOG_MAX_FILE_SIZE_MB` | `10` | When the ECS_LOG_ROLLOVER_TYPE variable is set to size, this variable determines the maximum size (in MB) the log file before it is rotated. If the rollover type is set to hourly then this variable is ignored. | `10` | `10` | | ||
| `ECS_LOG_MAX_FILE_SIZE_MB` | `1` | When the ECS_LOG_ROLLOVER_TYPE variable is set to size, this variable determines the maximum size (in MB) the log file before it is rotated. If the rollover type is set to hourly then this variable is ignored. | `1` | `1` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need to be changed to 5mb to reflect the defaults below
harishxr
previously approved these changes
Sep 23, 2025
mye956
previously approved these changes
Sep 23, 2025
- Change DEFAULT_ROLLOVER_TYPE from 'date' to 'size' - Change DEFAULT_MAX_FILE_SIZE from 10MB to 5MB - Update README.md documentation for ECS_LOG_ROLLOVER_TYPE and ECS_LOG_MAX_FILE_SIZE_MB - Fix unit tests to expect size-based rollover instead of date-based - Add test cases for explicit hourly rollover to maintain backward compatibility The reason for this change is to improve the situation where customers are pulling logs more than 24 hours after an issue has occured, and we are not able to troubleshoot it because there are no relevant logs available.
395818c
to
d38da04
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The reason for this change is to improve the situation where customers
are pulling logs more than 24 hours after an issue has occured, and we
are not able to troubleshoot it because there are no relevant logs
available.
Some metrics were pulled from our functional testing account to determine that this change should generally not negatively affect log retention in the case of a high-load instance, even in the case that debug logs are enabled.
With "debug" log level:
With "info" log level:
Testing
New tests cover the changes: no, modified existing unit tests
Description for the changelog
Enhancement: default to size-based log file rollover instead of hourly
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.